@derduher - We have a choice here. We can merge this to show the isolated test / fix OR we can merge a larger fix that addresses other issues with SitemapAndIndexStream and is easier to understand / maintain.

@huntharo I'm about to release a version that will be the last before a breaking change that drops support for node < 18. So if you have a preference now is the time to voice it :)

@huntharo I'm about to release a version that will be the last before a breaking change that drops support for node < 18. So if you have a preference now is the time to voice it :)

We should probably get these changes out before the node 12, 14, 16 drop.

@derduher You can proceed with dropping Node 12 support. The stream handling in Node 12 is pretty bad so one of the tests fails with Node 12 and can't be made to work.

I'm still reviewing this but in running npm run test:perf -- 3 3 stream true This MR has dropped peak memory usage by 1000x. from 78mb to 78kb. I had to modify the script to drop down a unit.

@huntharo thank you! Also be sure to see my comment above about the improved memory performance.

npm run test:perf -- 3 3 stream true

Hmm... I'm running that test but I'm not seeing a change in memory usage?

Yeah I don't think this PR changes anything that could impact that test?

yeah I think you are right. After merging the improvement in memory is gone. 🤷

 npm run test:perf -- 3 3 stream true     

> [email protected] test:perf
> node ./tests/perf.js 3 3 stream true

npm run test:perf -- [number of runs = 10] [batch size = 10] [stream(default)|combined] [measure peak memory = false]
runs: 3 batches: 3 total: 9
testing stream
========= stream =============
median: 0.0±0.0mb
99th percentile: 0.0mb

Now I'm getting the improvement, for the first time that I have observed. We should try to figure out what is going on here.

Key point: the imports in perf.js are from ../dist/ so they will be the last npm run build version of the code, not the latest .ts version of the code. This makes it possible to misattribute the fix to the wrong code. It's possible some other recent PR fixed this. We should be able to do a git bisect to find it.

Added #436 - The change is due to Node v18 -> v20... not due to any code change. Not sure yet if it is a change in memory reporting or a change in memory use.